-
Notifications
You must be signed in to change notification settings - Fork 228
fix(action menu): keyboard accessibility omnibus #5031
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
🦋 Changeset detectedLatest commit: b7398ad The changes in this PR will be included in the next version bump. Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
Tachometer resultsCurrently, no packages are changed by this PR... |
Lighthouse scores
What is this?Lighthouse scores comparing the documentation site built from the PR ("Branch") to that of the production documentation site ("Latest") and the build currently on Transfer Size
Request Count
|
Pull Request Test Coverage Report for Build 13553164764Details
💛 - Coveralls |
…child WAI ARIA APG
…nikkimk/fix-menu-a11y
@@ -135,4 +135,32 @@ export class ActionMenu extends ObserveSlotPresence( | |||
} | |||
super.update(changedProperties); | |||
} | |||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Action Menu was throwing the no label warning for a test that used the label-only
slot, so I decided to override Picker's accessible label check and warning with a warning specific to action menu.
…nikkimk/fix-menu-a11y
This issue seems resolved.
This issue seems resolved.
This issue is behaving better, but still a bit odd. When the submenu opens after clicking "Select some items" from the tray menu, VoiceOver on iOS seems to focus the items in the submenu, but the submenu appears to be hidden offscreen to the right, as if the menu were rendered using a forced popover. Within a tray, submenus should probably behave as they currently behave in React-Spectrum: https://react-spectrum.adobe.com/react-spectrum/Menu.html#dynamic, where the submenu slides in to replace the parent menu within the tray, and includes a button back to the parent menuitem at the top, in order to dismiss the submenu. We could treat the submenu with in a tray design as a separate concern.
This seems to be resolved. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
See comment: #5031 (comment)
Rajdeep approved after Tarun's day ended, unblocking reviews
Branch previewReview the following VRT differencesWhen a visual regression test fails (or has previously failed while working on this branch), its results can be found in the following URLs:
If the changes are expected, update the |
6251f5f
to
a99d776
Compare
* fix(menu): added rovingTabindexController and removed aria-activedescendant * fix(menu): fixes firefox keyboard navigation and call stack issues * test(menu): updates menu test based on changes * test(menu): adds click test for accessibility * test(menu): menu itself should delegate focus to active item * fix(menu): fixed menu group navigation according WAI ARIA APG * fix(reactive-controllers): complex element delegates focus to active child WAI ARIA APG * fix(menu): components should delegate focus * fix(menu): update keyboard nav to match WAI ARIA APG * fix(menu): updated docs to reflect preferred accessible method * fix(menu): selection and keyboard navigation according to APG * test(menu): updated to recommended a11y behavior from APG * docs(menu): reverted docs but fixed typos * fix(menu): removed unused map * test(menu): updated test to match changed group id * test(menu): keyboard selection is fixed * test(menu): updated to reflect WAI ARIA APG * fix(picker): picker should delegate focus * fix(action-menu): fixed a11y error message * fix(picker): according to APG ArrowDown should focus on first item * test(action-menu): updated tests for focus delegation and APG keyboard nav * fix(picker): ensure overlay is open before setting focus * fix(menu): removed extra focus * chore: added changeset * fix(picker): prevents the picker button from getting focus when menu is first updated * fix(menu): fixes preventDefault behavior when Space is pressed * fix(menu): accounted for action menus with no selection * fix(picker):sets focus on first selected item * fix(picker): focuses on 1st selected item when opened via kbd * test(picker): updated tests based on APG kbd nav * fix(menu): when I menuitem with a submenu closes * fix(picker): leverages RTI for ArrowLeft/ArrowRight * fix(picker): leverages RTI for ArrowLeft/ArrowRight * test(picker): adjusted tests based on a11y requirements * fix(picker): fixed accessibility warning * fix(picker): resolves opening and closing tests * fix(menu): resolves test by setting focus after scrolling * fix(picker): resolves mobile test * test(menu): updated tests for focus delegation * fix(picker): ensures that slotted labels don't get a11y warning * fix(picker): resolves menu close issue * fix(menu): fixed prev and next item * fix(picker): fixed left-right arrow keys * test(tray): debugging VO/iOS tray * fix(menu): resolves submenu tests * fix(menu): added some temporary debugging logic and comments * test(action-menu): removed test that no longer applies to a11y * fix(action-menu): fixed menu closing issue, added docs, and removed temporary debugging * test(action-menu): fixed test based on focus behavior when an item is selected * merge: merge from main * merge: merge from main * test(combobox): removing aria-activedescendant references until a combobox solution is implemented * test(combobox): removing aria-activedescendant references until a combobox solution is implemented * test(combobox): added skipped tests back * fix(combobox): resolves timing issues with updated menu * test(combobox): updated test docs * chore(combobox): removed debugging * docs(combobox): updated webkit escape hatches * test(combobox): removed isWebkit since escape hatch is not needed * test(tabs): removed test that does not follow WAI ARIA APG * fix(controllers): delegates focus setting until all other components using controllers are reviewed for a11y, * fix(controllers): fix(controllers): delegates focus setting until all other components using controllers are reviewed for a11y, * test(action-menu): fixed typo * test(action-menu): fixed typo * chore: fix menu tests with comments for skips * chore: cleanup * chore: rebuilt stylesheet * chore: dialog stylesheet * chore: dialog stylesheet * chore: modified pre-commit hook temporarily and fix spectrum-dialog.css * chore: revert hook * fix(menu): ensures menu used focus delegation * fix(action-group): should delegate focus * fix(action-group): prevents submenu events from reaching action group * fix(action-group): prevents submenu events from bubbling * fix(action-group): preventing submenu actions from being handled by action group * fix(action-menu): prevents submenu key events from being handle by action group * chore: added tons of comments to picker test * chore: added comments to action menu test * chore: added comments to action group test * fix(picker): fixed focus delegation * fix(menu): allows Escape to bubble to action menu and picker * fix(picker): fixes escape key and disabled closing * fix(picker): fixes escape key and disabled closing * test(menu): added more info to submenu test * fix(reactive-controllers): fixed failing reactive-controller tests * fix(picker): resolves focus styles in VRT * docs(tray): added a story to test buttons in tray on mobile * docs(tray): removed unnecessary story * docs(action-menu): added an example for #4623 * fix(menu): fixes for menu focus issues on chromium as well as updating menuitems * fix(menu): resolved all but a WebKit timeout on removal * chore: reverting last commit * fix(menu): fixes failing tests related to mutations * chore: styles from update from main * test(menu): removed skip from removal test * fix(combobox): fixes combobox focus issue * chore: updated CSS * chore: updated golden hash * fix(menu): menu item removal and focus * fix(menu): fixed aria-expanded in submenus * fix(menu): ensures sunmenu recieves focus when opened * fix(picker): ensures focus is not lost when overlay closes * fix(menu): resolves menu removal issue * chore: updated hash for new stories and a11y behaviors * fix(action-menu): resolves action-menu timeout * test(picker): fixed test based on a11y focus * test(picker): fixed timeout on picker test * test(action-menu): replaced removed test * chore(picker): fix missing import from merge * chore(picker): fixed merging issue * chore: reverting hash to main for comparison * fix(picker): focus should be managed by user not initial state * fix(picker): picker overlay should only focus with a user action * fix(picker): picker overlay should only focus with a user action * test(picker): fix timeout * test(picker): visual diffs because focus should only be set by user action * fix(menu): made submenu items focusable after open * fix(menu): fixes menuitem focus * test(menu): temporarily skipping Chrome * chore: reverting to main has for comparison * fix(menu): fixed menu test * fix(menu): fixed menu test * chore: updated golden images hash * fix(menu): fixes VoiceOver submenu focus Issue * fix(picker): retains focus after user closes menu * test(picker): uncommented tests * test(picker): fixed focus throw test * test(picker): removed `only` * test(picker): added test for retaining focus * chore: resolved hash conflict * chore: reverting hash to resolve * chore: updating hash * chore: reverting hash * chore: reverting hash * chore: reverting hash * chore: updating hash again * test(action-menu): added tests for returning focus * test(action-menu): added a11y test * test(action-menu): added a11y test * tes(action-menu): removed an `only` * test(action-menu): fixed linting issue --------- Co-authored-by: Casey Eickhoff <[email protected]>
* fix(menu): added rovingTabindexController and removed aria-activedescendant * fix(menu): fixes firefox keyboard navigation and call stack issues * test(menu): updates menu test based on changes * test(menu): adds click test for accessibility * test(menu): menu itself should delegate focus to active item * fix(menu): fixed menu group navigation according WAI ARIA APG * fix(reactive-controllers): complex element delegates focus to active child WAI ARIA APG * fix(menu): components should delegate focus * fix(menu): update keyboard nav to match WAI ARIA APG * fix(menu): updated docs to reflect preferred accessible method * fix(menu): selection and keyboard navigation according to APG * test(menu): updated to recommended a11y behavior from APG * docs(menu): reverted docs but fixed typos * fix(menu): removed unused map * test(menu): updated test to match changed group id * test(menu): keyboard selection is fixed * test(menu): updated to reflect WAI ARIA APG * fix(picker): picker should delegate focus * fix(action-menu): fixed a11y error message * fix(picker): according to APG ArrowDown should focus on first item * test(action-menu): updated tests for focus delegation and APG keyboard nav * fix(picker): ensure overlay is open before setting focus * fix(menu): removed extra focus * chore: added changeset * fix(picker): prevents the picker button from getting focus when menu is first updated * fix(menu): fixes preventDefault behavior when Space is pressed * fix(menu): accounted for action menus with no selection * fix(picker):sets focus on first selected item * fix(picker): focuses on 1st selected item when opened via kbd * test(picker): updated tests based on APG kbd nav * fix(menu): when I menuitem with a submenu closes * fix(picker): leverages RTI for ArrowLeft/ArrowRight * fix(picker): leverages RTI for ArrowLeft/ArrowRight * test(picker): adjusted tests based on a11y requirements * fix(picker): fixed accessibility warning * fix(picker): resolves opening and closing tests * fix(menu): resolves test by setting focus after scrolling * fix(picker): resolves mobile test * test(menu): updated tests for focus delegation * fix(picker): ensures that slotted labels don't get a11y warning * fix(picker): resolves menu close issue * fix(menu): fixed prev and next item * fix(picker): fixed left-right arrow keys * test(tray): debugging VO/iOS tray * fix(menu): resolves submenu tests * fix(menu): added some temporary debugging logic and comments * test(action-menu): removed test that no longer applies to a11y * fix(action-menu): fixed menu closing issue, added docs, and removed temporary debugging * test(action-menu): fixed test based on focus behavior when an item is selected * merge: merge from main * merge: merge from main * test(combobox): removing aria-activedescendant references until a combobox solution is implemented * test(combobox): removing aria-activedescendant references until a combobox solution is implemented * test(combobox): added skipped tests back * fix(combobox): resolves timing issues with updated menu * test(combobox): updated test docs * chore(combobox): removed debugging * docs(combobox): updated webkit escape hatches * test(combobox): removed isWebkit since escape hatch is not needed * test(tabs): removed test that does not follow WAI ARIA APG * fix(controllers): delegates focus setting until all other components using controllers are reviewed for a11y, * fix(controllers): fix(controllers): delegates focus setting until all other components using controllers are reviewed for a11y, * test(action-menu): fixed typo * test(action-menu): fixed typo * chore: fix menu tests with comments for skips * chore: cleanup * chore: rebuilt stylesheet * chore: dialog stylesheet * chore: dialog stylesheet * chore: modified pre-commit hook temporarily and fix spectrum-dialog.css * chore: revert hook * fix(menu): ensures menu used focus delegation * fix(action-group): should delegate focus * fix(action-group): prevents submenu events from reaching action group * fix(action-group): prevents submenu events from bubbling * fix(action-group): preventing submenu actions from being handled by action group * fix(action-menu): prevents submenu key events from being handle by action group * chore: added tons of comments to picker test * chore: added comments to action menu test * chore: added comments to action group test * fix(picker): fixed focus delegation * fix(menu): allows Escape to bubble to action menu and picker * fix(picker): fixes escape key and disabled closing * fix(picker): fixes escape key and disabled closing * test(menu): added more info to submenu test * fix(reactive-controllers): fixed failing reactive-controller tests * fix(picker): resolves focus styles in VRT * docs(tray): added a story to test buttons in tray on mobile * docs(tray): removed unnecessary story * docs(action-menu): added an example for #4623 * fix(menu): fixes for menu focus issues on chromium as well as updating menuitems * fix(menu): resolved all but a WebKit timeout on removal * chore: reverting last commit * fix(menu): fixes failing tests related to mutations * chore: styles from update from main * test(menu): removed skip from removal test * fix(combobox): fixes combobox focus issue * chore: updated CSS * chore: updated golden hash * fix(menu): menu item removal and focus * fix(menu): fixed aria-expanded in submenus * fix(menu): ensures sunmenu recieves focus when opened * fix(picker): ensures focus is not lost when overlay closes * fix(menu): resolves menu removal issue * chore: updated hash for new stories and a11y behaviors * fix(action-menu): resolves action-menu timeout * test(picker): fixed test based on a11y focus * test(picker): fixed timeout on picker test * test(action-menu): replaced removed test * chore(picker): fix missing import from merge * chore(picker): fixed merging issue * chore: reverting hash to main for comparison * fix(picker): focus should be managed by user not initial state * fix(picker): picker overlay should only focus with a user action * fix(picker): picker overlay should only focus with a user action * test(picker): fix timeout * test(picker): visual diffs because focus should only be set by user action * fix(menu): made submenu items focusable after open * fix(menu): fixes menuitem focus * test(menu): temporarily skipping Chrome * chore: reverting to main has for comparison * fix(menu): fixed menu test * fix(menu): fixed menu test * chore: updated golden images hash * fix(menu): fixes VoiceOver submenu focus Issue * fix(picker): retains focus after user closes menu * test(picker): uncommented tests * test(picker): fixed focus throw test * test(picker): removed `only` * test(picker): added test for retaining focus * chore: resolved hash conflict * chore: reverting hash to resolve * chore: updating hash * chore: reverting hash * chore: reverting hash * chore: reverting hash * chore: updating hash again * test(action-menu): added tests for returning focus * test(action-menu): added a11y test * test(action-menu): added a11y test * tes(action-menu): removed an `only` * test(action-menu): fixed linting issue --------- Co-authored-by: Casey Eickhoff <[email protected]>
Action menu items are not reading for screen readers.
Description
Action menu should be using a roving tabindex not
aria-activedescendant
because of cross-root ARIA limitations as well as lack of iOS support.The
sp-menu
that action menu uses was refactored to use a roving tabindex, and the numpad keys fix that was made in action menu are now applied to the focus group controller which the roving tabindex controller uses.Related issue(s)
Motivation and context
VoiceOver could not read the menu items when navigated via keyboard because of the cross-root aria issues above. Using the same roving tabindex controller that other components in our repo use, allows us to ensure roving tabindex and keyboard navigation is accessible and consistent across all components.
How has this been tested?
selects
and menu groups should function like the menu groups in the Editor Menubar Example from the WAI ARIA APGDoes screenreader read menuitems? (resolves #4556 and without regression on #3751)
Does picker menuitems selection work in VoiceOver on iOS (resolves #3774)
Can you use a screenreader to click a menuitem? (resolves #4997)
Does keyboard navigation of menuitems work as it should? (resolves #4532 and #4557)
Do menu groups avoid nested menus? (resolves #4614)
Types of changes
Checklist
Best practices
This repository uses conventional commit syntax for each commit message; note that the GitHub UI does not use this by default so be cautious when accepting suggested changes. Avoid the "Update branch" button on the pull request and opt instead for rebasing your branch against
main
.